Skip to content

fix(table): keep rows rendered when pageSizes prop changes at runtime - #1157

Merged
agupta-eightfold merged 2 commits into
mainfrom
agupta/table-pagesizes-resize-empty-fix
Jul 27, 2026
Merged

fix(table): keep rows rendered when pageSizes prop changes at runtime#1157
agupta-eightfold merged 2 commits into
mainfrom
agupta/table-pagesizes-resize-empty-fix

Conversation

@agupta-eightfold

@agupta-eightfold agupta-eightfold commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

SUMMARY:

Issue - If Table's internal pageSize is no longer present in an updated pagination.pageSizes prop, the pageData lookup finds no match and returns null. The table renders "No data found" with data still loaded, and only a remount recovers it.

How it happens - Pagination's mount effect persists the initial size into Table's inner pagination state, so any consumer that recomputes pageSizes later hits the mismatch. This is what blanked the Position Overview drill-in lists in vscode on window resize (found during ENG-203034 verification; product-side fix in EightfoldAI/vscode#113083).

Fix - In usePagination, normalize the merged pageSize to pageSizes[0] when pageSizes no longer contains it. In Table's pageData, replace the return-null fallthrough with a slice using the first size.

GITHUB ISSUE (Open Source Contributors)

NA

JIRA TASK (Eightfold Employees Only):

https://eightfoldai.atlassian.net/browse/ENG-205018

CHANGE TYPE:

  • Bugfix Pull Request
  • Feature Pull Request

TEST COVERAGE:

  • Tests for this change already exist
  • I have added unittests for this change

TEST PLAN:

  • New unit test in Table.pagination.test.js - mounts with pageSizes: [10], re-renders with pageSizes: [9], asserts rows still render. Fails without the fix (0 rows).
  • Table and Pagination suites pass (22 suites, 243 tests); the pre-commit hook runs the full repo suite.
  • No behavior change when pageSize is still valid - normalization only engages on the mismatch.

@codesandbox-ci

codesandbox-ci Bot commented Jul 24, 2026

Copy link
Copy Markdown

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 50.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.07%. Comparing base (f22fb38) to head (2cc4d4a).

Files with missing lines Patch % Lines
src/components/Table/Table.tsx 0.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1157      +/-   ##
==========================================
+ Coverage   85.00%   85.07%   +0.06%     
==========================================
  Files        1230     1230              
  Lines       21587    21593       +6     
  Branches     8209     8211       +2     
==========================================
+ Hits        18351    18371      +20     
+ Misses       3150     3136      -14     
  Partials       86       86              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@agupta-eightfold
agupta-eightfold force-pushed the agupta/table-pagesizes-resize-empty-fix branch from 792a9c2 to 287ef4c Compare July 27, 2026 08:15
Pagination's mount effect persists the initial page size into Table's
internal pagination state. If a consumer later passes a different
pagination.pageSizes (e.g. derived from the viewport height), the stale
internal pageSize no longer matches any entry: the pageData memo's lookup
loop falls through to return null and the table renders its empty state
("No data found") with data still present, recovering only on remount.

Two-layer fix:
- usePagination: normalize a merged pageSize that is not contained in the
  merged pageSizes to pageSizes[0], keeping the data slice, pager count,
  and currentPage clamp consistent.
- Table pageData memo: replace the terminal return-null fallthrough with a
  defensive slice using the first available size.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@agupta-eightfold
agupta-eightfold force-pushed the agupta/table-pagesizes-resize-empty-fix branch from 287ef4c to 70863f1 Compare July 27, 2026 12:35
@agupta-eightfold
agupta-eightfold merged commit fac8548 into main Jul 27, 2026
6 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants